Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Flipp: use height value from server response #3940

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Flipp: use height value from server response #3940

wants to merge 2 commits into from

Conversation

MishelleBit
Copy link

@MishelleBit MishelleBit commented Sep 25, 2024 •
edited
Loading

Using standardHeight and compactHeight from server response. Modified some tests accordingly as well.

@MishelleBit
OFF-1457: height should use the compactHeight and standardHeight fiel…
70dec6e 
…ds in decisions response (#8)

* add compactHeight and standardHeight fields in decision response

* modify tests

* modify tests

* modify tests

* modify tests

* adding height to expectedBidResponses

* final testing

* final changes

* final changes

* final changes
@github-actions GitHub Actions
Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 70dec6e

flipp

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:49:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:57:	MakeRequests		100.0%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:75:	makeRequest		75.0%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:95:	processImp		88.6%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:171:	buildPrebidRequest	100.0%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:188:	MakeBids		85.7%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:226:	getAdTypes		100.0%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:233:	buildBid		100.0%
github.com/prebid/prebid-server/v2/adapters/flipp/flipp.go:263:	paramsUserKeyPermitted	80.0%
total:								(statements)		89.3%

@MishelleBit MishelleBit changed the title flipp: change height to use compactHeight and standardHeight fields and modify tests accordingly Flipp: use height value from server response Sep 25, 2024
@MishelleBit
Copy link
Author

Hi, could I please get two reviews to proceed with this change? Thank you @przemkaczmarek @bsardo @onkarvhanumante @minaguib @dlackty @dmitris @asweeney86 @hhhjort

@przemkaczmarek przemkaczmarek self-assigned this Oct 10, 2024
@MishelleBit
Copy link
Author

Hi @przemkaczmarek, I hope you're doing well. Can we get an update on this?

@MishelleBit
Copy link
Author

Hello, could I please get another review to proceed with this change? Thank you @bsardo @onkarvhanumante @minaguib @dlackty @dmitris @asweeney86 @hhhjort

@bsardo bsardo self-assigned this Oct 24, 2024
@MishelleBit
Copy link
Author

Hi @bsardo , I hope you're doing well. Can we get an update on this?

@bsardo
Copy link
Collaborator

bsardo commented Nov 4, 2024

Hi @MishelleBit, we recently released PBS 3.0, more specifically v3.1.0, which updates Prebid Server package import references throughout the project from v2 to v3.
For example:

import (
    "github.com/prebid/prebid-server/v3/adapters"
)

As a result, please merge with master (no rebase) and then ensure all Prebid Server package import references in the files you’ve changed are v3 such that the test suite passes so we can resume reviewing. Thanks!

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 5, 2024

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 18cf454

flipp

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:50:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:58:	MakeRequests		100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:76:	makeRequest		75.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:96:	processImp		88.6%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:172:	buildPrebidRequest	100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:189:	MakeBids		85.7%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:227:	getAdTypes		100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:234:	buildBid		100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:264:	paramsUserKeyPermitted	80.0%
total:								(statements)		89.3%

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 5, 2024

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 18cf454

flipp

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:50:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:58:	MakeRequests		100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:76:	makeRequest		75.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:96:	processImp		88.6%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:172:	buildPrebidRequest	100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:189:	MakeBids		85.7%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:227:	getAdTypes		100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:234:	buildBid		100.0%
github.com/prebid/prebid-server/v3/adapters/flipp/flipp.go:264:	paramsUserKeyPermitted	80.0%
total:								(statements)		89.3%

@MishelleBit
Copy link
Author

Hi @bsardo, done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@przemkaczmarek przemkaczmarek przemkaczmarek approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@bsardo bsardo

@przemkaczmarek przemkaczmarek

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MishelleBit @bsardo @przemkaczmarek